-
Notifications
You must be signed in to change notification settings - Fork 4
Pass initial glucose concentration as parameter for metabolism modules #96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… metabolism classes, adding default parameter value to parameters.xml
| private final double atpProductionRate; | ||
|
|
||
| /** Initial cell internal glucose concentration [fmol]. */ | ||
| private final double initGluc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since initGluc is only used in the constructor, I think you can just inline the variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed !
jessicasyu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
cainja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that this value represents a concentration...
src/arcade/patch/parameter.patch.xml
Outdated
| <population.process process="metabolism" id="CONSTANT_GLUCOSE_UPTAKE_RATE" value="929.88" unit="fmol glucose/min/M glucose" description="constant glucose uptake rate" /> | ||
| <population.process process="metabolism" id="CONSTANT_ATP_PRODUCTION_RATE" value="4.9817" unit="fmol ATP/cell/min" description="constant ATP production rate" /> | ||
| <population.process process="metabolism" id="CONSTANT_VOLUME_GROWTH_RATE" value="2.819" unit="um^3/min" description="constant volume growth rate"/> | ||
| <population.process process="metabolism" id="INITIAL_GLUCOSE_CONCENTRATION" value="0" unit="fmol" description="initial cell internal glucose concentration"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't love this being 0 by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What default values would you suggest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this value being 0 the source of the bug that you had identified before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah - it probably makes sense to set it to the average(?) internal glucose amt of engineered cells, Im just not sure what that is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now Ill set it as 30fmol since thats what CARCADE seems to be giving me, but we can revisit this value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@allison-li-1016 @cainja Possible alternative: keep the parameter as a concentration that matches the glucose concentration and do the conversion to mass in the constructor by multiplying by cell volume?
…lying concentration by volume to get glucose mass
closes #93
Estimated review time
small
Summary
Allows for initial internal glucose concentrations to be passed in as a parameter. By default, this value is set to 0.
Description of changes
Description of how to review changes